First weekend final version #13
Conversation
… menu... can add to current order by pushing button below food choice
…get remove button working
|
|
||
| const menu = { | ||
| 1 : { | ||
| id: Math.floor(Math.random() * (max - min + 1)) + min, |
There was a problem hiding this comment.
It would make more sense to use the same id as was used for the key here. Otherwise we end up with inconsistent ids on every executing. Also, by using random we run the risk of duplicate ids which may cause strange bugs
|
|
||
| // #3 Works | ||
| getMenuItem(id) { | ||
| const foodItem = Object.values(menu).find( item => item.id === parseInt(id)); |
There was a problem hiding this comment.
Since we are using ids as keys in menu object, we can get individual menu items using menu[id]
| if (result) { | ||
| res.json(result); | ||
| } else { | ||
| res.status(404).send('The food item with the given ID was not found'); |
There was a problem hiding this comment.
It would better the send the error message as JSON so that it can be read by fetch the same way as normal result. Otherwise, the front end will throw an error when it tries to convert a string to object.
| @@ -0,0 +1,90 @@ | |||
|
|
|||
| orderList = [ | |||
There was a problem hiding this comment.
It would be better to store orders as an object rather than as an array so we can look up orders easily using ids
|
|
||
|
|
||
| getFetch() { | ||
| const serverFetch = `http://localhost:8080/menu` |
There was a problem hiding this comment.
it would better use relative url here /menu as that would allow the code to work when app is deployed to server with another url
| } | ||
|
|
||
|
|
||
| getFetch() { |
There was a problem hiding this comment.
getFetch is a little ambiguous, something like fetchMenu would work better as its more descriptive of the function behaviour
|
|
||
| const min = 1; | ||
| const max = 100; | ||
| let keyId = Math.floor(Math.random() * (max - min + 1)) + min; |
There was a problem hiding this comment.
It would better to generate sequential ids on server to avoid potential duplicates that can result from random generation or multiple users generating same id
| const newTotalPrice = Number(this.state.totalPrice) + order.price; | ||
| console.log("newTotalPrice",newTotalPrice); | ||
| this.setState( { | ||
| currentOrder : order, |
There was a problem hiding this comment.
It's not clear how currentOrder is used. It is initialised as an array, but get's an object set in it. Also it looks like currentOrder is not used in the application
|
Good work. There are few bits that need a little tidying up, but overall it looks like a great start |
No description provided.